Skip to content

feat(kotlin-sdk)!: keystore rework — policy-alias split, layered key recovery, durable repair, structured signer errors (stacked on #4191) - #4183

Merged
QuantumExplorer merged 32 commits into
dashpay:v4.2-devfrom
bfoss765:port/v4.1/android-integration
Aug 4, 2026
Merged

Conversation

@bfoss765

@bfoss765 bfoss765 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Stacked on #4191 (port/v4.1/unmanaged-identity-reads) — review only the
commits above ce3a17d71f. Once #4191 merges,
git rebase --onto origin/v4.1-dev ce3a17d71f and force-push with
--force-with-lease.

This replaces the old single-squash port/v4.1/android-integration
(preserved at backup/port-4060-pre-rework). That squash predated #4172 and
silently rewrote the exact regions #4172 later hardened; this branch is a
rebuild on the current base with each review finding resolved explicitly.
The unmanaged-identity reads and the text-classified SigningKeyUnavailable
from the old squash shipped separately as #4191 and are NOT duplicated here.

Scope — Keystore only, one logical commit per concern:

  1. Alias split + policyKeySecurityPolicy (AUTH_GATED default /
    DEVICE_BOUND opt-in), two dedicated RSA aliases, legacy KEYS_ALIAS
    read-only; fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172's fingerprint capture + invalidation recovery survive
    alias-parameterized (finding 1).
  2. Lockless degradation, honest — AUTH_GATED writes on a lockless device
    redirect to the DEVICE_BOUND alias (never a silently gate-less key under
    the auth-gated alias); effectiveKeySecurityPolicy() +
    requireAuthGated strict mode; per-blob alias tag routes reads;
    tightened KeyMint failure classifier (finding 4).
  3. Layered retrieve ladder — legacy shape-dispatch FIRST (the fp gate
    never applies to legacy blobs, finding 2), fingerprint fast path with
    KPIE rethrow-after-generation-checked-cleanup (brick-loop fix, finding 1),
    mismatch/missing fp routes into former-RSA recovery, migration forward.
    Cheap canSignWith never decrypts; the real-decrypt probe moved to
    probeIdentityKeyRecoverability (finding 3).
  4. Pending keys — queryable pendingIdentityKeys StateFlow,
    round-transactional staging.
  5. Durable repair signal — Room 7→8 derivation-breadcrumb columns;
    restart reconstruction incl. blobs stranded by keypair replacement
    (finding 5).
  6. Forced, verified repairreplacePrivateKey + force derive path +
    real-decrypt verification before markIdentityKeyRepaired (finding 6).
  7. ⚠ BREAKING: 98 → PlatformWallet.NotFound (Swift parity; release
    note in the commit).
  8. Structured signer discriminator — typed error_code on the sign
    completion ABI, ErrorSigningKeyUnavailable = 31 (26-30 reserved for
    feat(kotlin-sdk): split build/broadcast with reservation release for BIP70-style deferred submission #4185/fix: shield asset-lock funding from all funds accounts incl. CoinJoin (#4073) #4184), Kotlin/Swift typed mappings; MESSAGE_MARKER sniff retained as a
    deprecated fallback for the fix(kotlin-sdk): unmanaged-identity reads return absence + typed SigningKeyUnavailable (split from #4183) #4191 merge-order transition (mixed
    old-native/new-Kotlin artifacts are unsupported — the completion JNI
    arity changed) (finding 7). Known residual: the
    Rust-internal segment rides a stable machine prefix through
    ProtocolError::Generic — a typed rs-dpp variant was deliberately
    avoided (serialization blast radius).
  9. Instrumented tests adapted to the policy alias (finding 8; the five
    InvalidKeyException failures were the pre-fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172 emulator-relock flake —
    inherited workflow guard covers them).
  10. Docs — parity manifest/spec entries (alias split N/A for iOS, 98 now
    convergent, pending-repair gap recorded for Swift, code-31 capability),
    migration ladder v8 note.

CI expectations: emulator job runs :sdk:connectedDebugAndroidTest (watch
the deviceLocked=0 guard); swift job rebuilds the xcframework so the
5-arg completion ABI and code 31 compile against fresh headers.


Round-3 addendum: verification passes found and fixed three repair-routing defects (health-probe fingerprint disproof for replaced aliases, typed first-attempt invalidation errors, durable repair seeding for legacy and former-RSA keys — commits 6d1fa5be83, f6fe865e56, 7ee6e4b7f9, ce3490a3a0). Unit suite: 249 passing; instrumented suite runs on this repo's emulator job.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added selectable identity-key security policies (auth-gated vs device-bound) and stricter policy enforcement on unsupported devices.
    • Introduced durable “pending identity keys” tracking with a restart-surviving repair eligibility probe.
    • Added typed “signing key unavailable” reporting for clearer, non-ambiguous signer failures.
  • Bug Fixes
    • Correctly distinguishes missing wallets from unmanaged-identity situations.
    • DashPay now treats “managed identity not found” as an absence (instead of surfacing an error).
  • Security
    • Improved identity-key generation/recovery and alias handling to avoid unsafe fallback behavior on lock-screen constrained devices.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@QuantumExplorer, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f042201-44f6-4b89-b7a7-46c6649cc70c

📥 Commits

Reviewing files that changed from the base of the PR and between 48c4726 and 6e076c0.

📒 Files selected for processing (34)
  • docs/dashpay/KOTLIN_MIGRATION_LEFTOVERS.md
  • docs/sdk/KOTLIN_SWIFT_SHARED_PARITY_SPEC.md
  • docs/sdk/sdk-parity-manifest.json
  • packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/wallet/WalletKeyHealthSheet.kt
  • packages/kotlin-sdk/PARITY_SUMMARY.md
  • packages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/8.json
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerInstrumentedTest.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/WalletStorageOwnershipTest.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/SignerNative.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/PublicKeyDao.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/PublicKeyEntity.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/Hash160.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/IdentityKeyPrivateKeyDeriver.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyUnavailableException.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/Hash160Test.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/IdentityKeyPrivateKeyDeriverMatchTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreKeyGenPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerCompletionCodeTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
  • packages/rs-platform-wallet-ffi/src/dashpay.rs
  • packages/rs-platform-wallet-ffi/src/error.rs
📝 Walkthrough

Walkthrough

Adds policy-based keystore aliases and migration handling, transactional pending identity-key tracking, structured signer errors, typed platform-wallet errors, and managed-identity lookup classification across Kotlin, Rust, and Swift.

Changes

Keystore policy and identity-key recovery

Layer / File(s) Summary
Policy-driven keystore aliases and generation
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/*, packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/*
Adds AUTH_GATED and DEVICE_BOUND policies, separate RSA aliases, alias-aware cryptography, and lock-screen degradation handling.
Wallet storage recovery and migration
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt, packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
Adds policy wiring, alias-tagged persistence, legacy AES/RSA recovery, conditional migration, and recoverability probing.

Pending identity-key repair state

Layer / File(s) Summary
Transactional pending-key tracking
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/*, packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/*
Records failed derivations, persists derivation breadcrumbs, stages pending-key changes through commits, preserves state on rollback, and reconstructs state after restart.
Pending-key repair exposure
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt, packages/kotlin-sdk/KotlinExampleApp/.../WalletKeyHealthSheet.kt
Exposes pending identity keys and performs forced repair after recoverability verification.

Signer and platform-wallet error flows

Layer / File(s) Summary
Structured signer completion errors
packages/rs-sdk-ffi/src/signer.rs, packages/rs-unified-sdk-jni/src/signer.rs, packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/{ffi,security}/*, packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift
Adds explicit signer error codes to completion callbacks and propagates signing-key-unavailable classifications across FFI boundaries.
Typed platform-wallet errors
packages/rs-platform-wallet-ffi/src/error.rs, packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt, packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
Adds typed signing-key-unavailable errors and maps platform-wallet code 98 to wallet-level NotFound.
Managed-identity outcome classification
packages/rs-platform-wallet-ffi/src/dashpay.rs
Separates invalid handles, removed wallets, unmanaged identities, and successful lookups.
DashPay not-found translation
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/{tokens,wallet,identity}/*, packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/*
Translates managed-identity not-found results to zero handles for local operations and rethrows unrelated errors.

Schema and parity artifacts

Layer / File(s) Summary
Room schema migration
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/{DashDatabase.kt,dao,entities}/*, packages/kotlin-sdk/sdk/schemas/.../8.json, packages/kotlin-sdk/sdk/src/androidTest/.../DashDatabaseMigrationTest.kt
Moves Room to schema version 8 and adds nullable derivation breadcrumb columns to public_keys.
Parity and migration documentation
docs/sdk/*, docs/dashpay/KOTLIN_MIGRATION_LEFTOVERS.md, packages/kotlin-sdk/PARITY_SUMMARY.md
Documents capability coverage, host differences, schema rollout, and deferred fallback removal.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Signer
  participant KotlinJNI
  participant RustFFI
  participant PlatformWallet
  Signer->>KotlinJNI: completeSign(errorCode, errorMessage)
  KotlinJNI->>RustFFI: forward structured signer error
  RustFFI->>PlatformWallet: map signing-key-unavailable prefix to code 31
  PlatformWallet-->>Signer: return typed platform-wallet error
Loading

Suggested reviewers: shumkov, bezibalazs, quantumexplorer

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main Kotlin SDK changes: keystore policy split, layered recovery, durable repair, and structured signer errors.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added this to the v4.1.0 milestone Jul 21, 2026
@thepastaclaw

thepastaclaw commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

🕓 Ready for review — 11 ahead in queue (commit 6e076c0)
Queue position: 12/15 · 2 reviews active
ETA: start ~06:49 UTC · complete ~07:06 UTC (median 17m across 30 recent reviews; 2 slots)
Queued 1h 6m ago · Last checked: 2026-08-04 05:10 UTC

@bfoss765

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt`:
- Around line 549-562: Update the migration block around store.edit in
WalletStorage so CancellationException is not swallowed by runCatching; rethrow
cancellation while retaining the existing handling for non-cancellation failures
and successful migration behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 987bcd6c-12ec-4a70-8f81-bdad970986df

📥 Commits

Reviewing files that changed from the base of the PR and between 8b466ab and 00804cd.

📒 Files selected for processing (15)
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreKeyGenPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.kt
  • packages/rs-platform-wallet-ffi/src/dashpay.rs

@shumkov

shumkov commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

This needs reconciliation with #4172 before it can merge — the branch is CONFLICTING, and the conflicts are semantic, not textual. Concretely:

  • fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172's KeyPermanentlyInvalidatedException recovery (generation-checked alias deletion) must survive: this PR's decrypt() rewrite lacks it, and with the alias split, repair would re-encrypt under the invalidated pair's still-present cert → permanent brick loop. (Your generateWithLockScreenDegradation actually complements it — both need to survive.)
  • fix(kotlin-sdk): harden identity-key Keystore recovery and reconcile parity docs #4172's fingerprint gate in retrievePrivateKey and this PR's legacy-blob recovery are mutually exclusive as written (legacy blobs by definition fail the fingerprint check). Needs a layered resolution: legacy shape-detect → recovery first; fingerprint fast-path for current blobs.
  • Auto-merge trap: v4.1-dev's canSignWith is contractually cheap/non-mutating on the Rust callback thread; this PR redefines isPrivateKeyDecryptable into a real decrypt probe (RSA op + runBlocking). Keep the cheap check on canSignWith; move the probing version to a key-health method.
  • Instrumented CI is red: storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced still deletes legacy KEYS_ALIAS but writes moved to KEYS_ALIAS_AUTH_GATED; five more failures need triage.
  • Sweep incomplete: ManagedPlatformWallet.kt:549 and IdentityRegistration.kt:93 keep the dead == 0L check — one unmanaged id now throws through the whole listing.

Suggestion: split the unmanaged-identity-reads + typed SigningKeyUnavailable changes (both good, conflict-free, wanted) into their own PR so they aren't blocked on the Keystore rework. Also: the 98→PlatformWallet.NotFound remap is host-breaking and needs a release-note flag; parity docs (#4172's manifest/spec) need updating for the new deliberate divergences.

@shumkov

shumkov commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Additional findings after checking the existing threads and the current #4172 base:

  1. AUTH_GATED silently loses authentication on lockless devices. KeystoreManager drops both lock-bound parameters but continues to use the AUTH_GATED alias/policy. The fallback classifier also accepts any nested KeyStoreException after the generic "key generation failed" marker. Either fail with a typed policy error or explicitly select DEVICE_BOUND; do not describe the resulting key as auth-gated.
  2. The repair signal is not durable. pendingIdentityKeys starts as an empty in-memory MutableStateFlow and is not reconstructed from Room. A normal restart loses the key slot needed for repair while the durable identity remains watch-only.
  3. Repair can report success without replacing the bad blob. storeIfAbsent treats RSA shape plus alias fingerprint as usable without performing a real decrypt. repairIdentityKey can then skip derivation and still call markIdentityKeyRepaired. Repair needs a forced atomic replacement path, followed by a real-decrypt check before clearing pending state.
  4. SigningKeyUnavailable is still text-classified. KeystoreSigner emits a marker and DashSdkError uses message.contains. Please carry a structured signer/native discriminator through Rust/C/JNI and map it in Kotlin and Swift.

I am not repeating the existing rebase/invalidation-recovery and cancellation comments; those remain valid. The earlier canSignWith decrypt concern is resolved at this head, and the legacy-before-fingerprint recovery is already present, so both should simply be preserved during conflict resolution.

@bfoss765

Copy link
Copy Markdown
Contributor Author

Took your suggestion: the conflict-free slices are split out as #4191 — unmanaged-identity reads (including both == 0L sites you caught, so one unmanaged id no longer fails a whole listing) plus the typed SigningKeyUnavailable, applied clean on current v4.1-dev with the Keystore rework and the host-breaking 98-remap deliberately excluded. This PR then becomes the Keystore rework only, properly reconciled with #4172's invalidation recovery and fingerprint gate; your four second-pass findings (lockless-device policy, repair durability, forced re-encrypt, structured discriminator) will be addressed there, with the 98→NotFound remap release-note-flagged and the instrumented failures triaged as part of the reconciliation.

@bfoss765
bfoss765 force-pushed the port/v4.1/android-integration branch from 00804cd to ce3490a Compare July 21, 2026 23:26
bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 21, 2026
…as split

The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and
the single-alias API; after the alias split the default-policy storage
writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by
deleting the legacy alias no longer invalidates anything (the CI failure
shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced).

- WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and
  no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the
  fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the
  captured producing alias); the stale-invalidation-cleanup test uses the
  alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration.
- KeystoreSignerInstrumentedTest: canSignWith rejection simulates
  replacement of the policy alias.

The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures
from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated
dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard
+ the hard deviceLocked=0 guard), so the emulator re-locked mid-run and
every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the
current base inherits the fixed workflow; those tests are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bfoss765 bfoss765 changed the title fix(kotlin-sdk): android host-app integration fixes (key security policy, unmanaged-identity reads, typed signing error) feat(kotlin-sdk)!: Keystore rework — policy-alias split, layered key recovery, durable repair, structured signer errors (stacked on #4191) Jul 21, 2026
@bfoss765 bfoss765 changed the title feat(kotlin-sdk)!: Keystore rework — policy-alias split, layered key recovery, durable repair, structured signer errors (stacked on #4191) feat(kotlin-sdk)!: keystore rework — policy-alias split, layered key recovery, durable repair, structured signer errors (stacked on #4191) Jul 21, 2026
@shumkov

shumkov commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Consolidated re-review of the rework (two independent passes). First the good news: all seven previously-blocking items are genuinely resolved — invalidated-key cleanup survives with generation-checked deletion, the retrieve ladder layers legacy shape-dispatch before the fingerprint gate, canSignWith is back to presence/fingerprint-only, the instrumented suite is retargeted and green, the #4191 stacking is clean, lockless devices get an honest effectiveKeySecurityPolicy(), and the typed code-31 signer error is a real upgrade.

However, the rework surfaces new blocking findings:

  1. Repair can persist the wrong private key. The repair path derives from the caller-supplied indices, and the example app passes keyIndex = key.keyId (the DPP key id) — even though PublicKeyEntity carries derivationIdentityIndex/derivationKeyIndex breadcrumbs for exactly this purpose. repairIdentityKey's verification is probeIdentityKeyRecoverability, which proves the stored blob decrypts — not that the derived key matches publicKeyData. A wrong index derives a different valid scalar that round-trips fine, clears pending state, and persists an unusable key. Fix: repair from the persisted derivation indices and verify derived-public-key equality against publicKeyData before persistence and state-clear. (WalletKeyHealthSheet.kt:173-178, PlatformWalletManager.kt:482-540, IdentityKeyPrivateKeyDeriver.kt:48-100)

  2. KeyMint failure classifier is too broad. isNoSecureLockScreenKeyGenFailure path A classifies any KeyStoreException whose message contains generate_key (KeystoreManager.kt:874-910) — a transient KeyMint generation failure on a device with a lock screen silently and permanently downgrades AUTH_GATED to DEVICE_BOUND. Narrow to the authoritative lock-screen signal (numeric code / "lock screen" text) and drop the bare generate_key match.

  3. Durable repair state fails open. In repairIdentityKey, the Room privateKeyKeychainIdentifier update is wrapped in runCatching (swallowed on failure) while markIdentityKeyRepaired clears live pending state regardless — a failed durable write resurrects the repair after restart while the session believes it's done. Make the durable update fail closed.

  4. The existing unresolved thread on CancellationException being caught and suppressed in the WalletStorage migration remains valid.

  5. Minor: DEVICE_BOUND documentation promises hardware-backed storage while the implementation permits the software AndroidKeyStore fallback — align the docs.

@bfoss765

Copy link
Copy Markdown
Contributor Author

All five new findings addressed (255 unit tests, 0 failures):

  1. Repair-key correctness — repair now derives from the persisted derivationIdentityIndex/derivationKeyIndex breadcrumbs (never caller-supplied indices) and verifies the derived public key equals publicKeyData before any store or state-clear; a mismatch throws IdentityKeyDerivationMismatchException and leaves pending intact. The example app no longer passes key.keyId as the index.
  2. Classifier — dropped the bare generate_key substring match; lockless classification now rests only on the authoritative lock-screen signal (text or numeric code 4/10309), so a transient KeyMint failure on a lock-screen device no longer permanently downgrades AUTH_GATED→DEVICE_BOUND.
  3. Durable repair fails closed — the privateKeyKeychainIdentifier write propagates on failure and markIdentityKeyRepaired runs only after it commits, so a failed write leaves pending intact and self-heals on restart.
  4. Cancellation — the WalletStorage migration rethrows CancellationException; only genuine rewrite failures stay best-effort.
  5. DEVICE_BOUND docs — corrected to document the StrongBox→TEE→software-AndroidKeyStore fallback rather than promising hardware backing.

Two honest residuals, both fail-closed (neither can persist a wrong key): the real deriver's FFI pubkey-equality is exercised on-device rather than in a JVM unit test (AndroidKeyStore crypto can't run on the JVM) — it's in our device test plan; and on API 29–32 the numeric error code isn't available, so an exotic "reports-secure-but-rejects" OEM quirk on old Android wouldn't be text-classified, though genuine lockless devices are still caught by the primary isDeviceSecure() probe.

@bfoss765
bfoss765 force-pushed the port/v4.1/android-integration branch from 8313192 to 48c4726 Compare July 23, 2026 01:10
bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 23, 2026
…otFound

Split out of dashpay#4183 (port of dashpay#4060) per review: the
unmanaged-identity read fixes are conflict-free on v4.1-dev and should
not be blocked on the Keystore rework.

The FFI's blanket Option -> result conversion reports an identity the
wallet does not manage as PlatformWalletFFIResultCode::NotFound (98),
so the Kotlin callers' zero-handle checks were dead and every local
read over an unmanaged identity threw instead of returning absence.

- Dashpay: route getManagedIdentity through
  translateManagedIdentityNotFoundToZero so contacts()/syncState()/
  payments()/sendContactRequest() treat "not managed" as null/empty/
  false; ErrorInvalidHandle still propagates.
- Sweep the remaining dead `== 0L` sites: ManagedPlatformWallet
  .inMemoryIdentityStates (one unmanaged/just-removed id no longer
  throws through the whole listing) and IdentityRegistration
  .contestedDpnsNames (the intended "identity is not managed by this
  wallet" NotFound now actually surfaces).
- platform-wallet-ffi: platform_wallet_get_managed_identity keeps the
  three outcomes distinct (classify_managed_identity_outcome) so a
  stale/removed wallet surfaces ErrorInvalidHandle and never
  masquerades as an unmanaged identity; unit tests pin both arms.
- DashSdkError: name the code (PLATFORM_WALLET_NOT_FOUND_CODE = 98);
  the 98 -> DashSdkError.NotFound mapping is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 23, 2026
Split out of dashpay#4183 (port of dashpay#4060) per review: the
typed signing error is conflict-free on v4.1-dev and should not be
blocked on the Keystore rework.

The KeystoreSigner "missing key" completion error travels as free text
through Rust and used to come back as an opaque PlatformWallet.Generic
(or WalletOperation) failure. It is now built from a shared
MESSAGE_MARKER constant and recognized on the Kotlin boundary as the
typed DashSdkError.PlatformWallet.SigningKeyUnavailable, so hosts can
route users to key repair instead of showing a generic error. The
marker is only consulted on the catch-all codes (6 / else), so the
dedicated retry-semantics types are never overridden.

Known limitation (kept as-is from dashpay#4183 by request): the discriminator
is message-text-based (message.contains on the marker); a structured
error code across the FFI boundary is follow-up work in the parent PR
line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 23, 2026
…as split

The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and
the single-alias API; after the alias split the default-policy storage
writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by
deleting the legacy alias no longer invalidates anything (the CI failure
shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced).

- WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and
  no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the
  fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the
  captured producing alias); the stale-invalidation-cleanup test uses the
  alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration.
- KeystoreSignerInstrumentedTest: canSignWith rejection simulates
  replacement of the policy alias.

The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures
from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated
dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard
+ the hard deviceLocked=0 guard), so the emulator re-locked mid-run and
every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the
current base inherits the fixed workflow; those tests are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt (1)

1001-1005: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

runCatching around a suspend call swallows CancellationException. reconstructPendingIdentityKeysFromPersistence(...) is a suspend function; runCatching catches Throwable, so if loadPersistedWallets is cancelled while this best-effort reconstruction is suspended, the cancellation is absorbed and the coroutine keeps going into the wallet-restore loop instead of unwinding. Given this PR already tightened CancellationException handling elsewhere, rethrow it here too.

♻️ Rethrow cancellation, swallow the rest
-        runCatching {
-            persistenceHandler.reconstructPendingIdentityKeysFromPersistence(
-                isPrivateKeyDecryptable = { walletStorage.isPrivateKeyDecryptable(it) },
-            )
-        }
+        try {
+            persistenceHandler.reconstructPendingIdentityKeysFromPersistence(
+                isPrivateKeyDecryptable = { walletStorage.isPrivateKeyDecryptable(it) },
+            )
+        } catch (c: kotlinx.coroutines.CancellationException) {
+            throw c
+        } catch (_: Throwable) {
+            // Best-effort: a reconstruction failure must not block wallet restore.
+        }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`
around lines 1001 - 1005, Update the runCatching block around
reconstructPendingIdentityKeysFromPersistence to rethrow CancellationException
while continuing to swallow other failures. Preserve the existing best-effort
behavior for non-cancellation exceptions and allow coroutine cancellation to
propagate before the wallet-restore loop continues.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/wallet/WalletKeyHealthSheet.kt`:
- Around line 94-102: Update the state passed from the probe block around
probeIdentityKeyRecoverability to represent key recoverability rather than
keystore-entry presence, and rename the corresponding property and usages in
KeyRow. Preserve the false result for failed probes, but render that state as an
unrecoverable/decryption failure instead of “no Keystore entry,” while retaining
the existing message for genuinely absent entries.
- Around line 94-102: Move the synchronous recoverability probing in the
key-health report producer off the composition thread by wrapping the
report-building work, including probeIdentityKeyRecoverability, in
withContext(Dispatchers.IO), then assign the completed report to produceState’s
value. Keep the existing probe results and fallback behavior unchanged.

---

Nitpick comments:
In
`@packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt`:
- Around line 1001-1005: Update the runCatching block around
reconstructPendingIdentityKeysFromPersistence to rethrow CancellationException
while continuing to swallow other failures. Preserve the existing best-effort
behavior for non-cancellation exceptions and allow coroutine cancellation to
propagate before the wallet-restore loop continues.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1d0971c9-bcbb-40ff-8147-c9ef5050c3d3

📥 Commits

Reviewing files that changed from the base of the PR and between ce3490a and 48c4726.

📒 Files selected for processing (50)
  • docs/dashpay/KOTLIN_MIGRATION_LEFTOVERS.md
  • docs/sdk/KOTLIN_SWIFT_SHARED_PARITY_SPEC.md
  • docs/sdk/sdk-parity-manifest.json
  • packages/kotlin-sdk/KotlinExampleApp/app/src/main/java/org/dashfoundation/example/ui/wallet/WalletKeyHealthSheet.kt
  • packages/kotlin-sdk/PARITY_SUMMARY.md
  • packages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/8.json
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabaseMigrationTest.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerInstrumentedTest.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/WalletStorageOwnershipTest.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/SignerNative.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/PublicKeyDao.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/PublicKeyEntity.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/IdentityKeyPrivateKeyDeriver.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyUnavailableException.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreManager.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/WalletStorage.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/PlatformWalletManager.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/errors/DashSdkErrorTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandlerTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreKeyGenPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerCompletionCodeTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.kt
  • packages/rs-platform-wallet-ffi/src/dashpay.rs
  • packages/rs-platform-wallet-ffi/src/error.rs
  • packages/rs-sdk-ffi/src/signer.rs
  • packages/rs-sdk-ffi/src/test_utils.rs
  • packages/rs-sdk-ffi/src/token/claim.rs
  • packages/rs-sdk-ffi/src/token/config_update.rs
  • packages/rs-sdk-ffi/src/token/destroy_frozen_funds.rs
  • packages/rs-sdk-ffi/src/token/emergency_action.rs
  • packages/rs-sdk-ffi/src/token/freeze.rs
  • packages/rs-sdk-ffi/src/token/mint.rs
  • packages/rs-sdk-ffi/src/token/purchase.rs
  • packages/rs-sdk-ffi/src/token/set_price.rs
  • packages/rs-sdk-ffi/src/token/transfer.rs
  • packages/rs-sdk-ffi/src/token/unfreeze.rs
  • packages/rs-unified-sdk-jni/src/signer.rs
  • packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
  • packages/swift-sdk/SwiftTests/SwiftDashSDKTests/ErrorHandlingTests.swift
🚧 Files skipped from review as they are similar to previous changes (36)
  • packages/swift-sdk/SwiftTests/SwiftDashSDKTests/ErrorHandlingTests.swift
  • packages/rs-sdk-ffi/src/test_utils.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/SignerNative.kt
  • packages/rs-sdk-ffi/src/token/emergency_action.rs
  • packages/rs-sdk-ffi/src/token/set_price.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/dao/PublicKeyDao.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerInstrumentedTest.kt
  • packages/rs-sdk-ffi/src/token/claim.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyUnavailableException.kt
  • packages/rs-sdk-ffi/src/token/config_update.rs
  • packages/rs-sdk-ffi/src/token/transfer.rs
  • packages/rs-sdk-ffi/src/token/unfreeze.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/identity/IdentityRegistration.kt
  • packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ManagedIdentityNotFoundTranslationTest.kt
  • docs/sdk/sdk-parity-manifest.json
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedPlatformWallet.kt
  • packages/rs-sdk-ffi/src/token/mint.rs
  • packages/rs-unified-sdk-jni/src/signer.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/DashDatabase.kt
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeystoreSignerCompletionCodeTest.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/entities/PublicKeyEntity.kt
  • packages/rs-platform-wallet-ffi/src/error.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicy.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.kt
  • packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift
  • packages/kotlin-sdk/PARITY_SUMMARY.md
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/KeySecurityPolicyTest.kt
  • packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/security/WalletStorageOwnershipTest.kt
  • packages/rs-platform-wallet-ffi/src/dashpay.rs
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/security/KeystoreSigner.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/tokens/Dashpay.kt
  • packages/kotlin-sdk/sdk/schemas/org.dashfoundation.dashsdk.persistence.DashDatabase/8.json
  • packages/rs-sdk-ffi/src/signer.rs
  • packages/kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/security/WalletStorageUpgradeMatrixTest.kt
  • packages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/persistence/PlatformWalletPersistenceHandler.kt

@shumkov

shumkov commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Round-3 verification (two independent passes, reconciled): all five previous findings are genuinely fixed — breadcrumb-driven repair with the index parameters removed from the API entirely, fail-closed durable writes, derived-pubkey verification before persistence, narrowed classifier, cancellation rethrow, honest DEVICE_BOUND docs. Two new findings from the deeper pass:

  1. P1 — repair permanently rejects valid ECDSA_HASH160 keys. derivedPublicKeyMatches is a raw contentEquals (IdentityKeyPrivateKeyDeriver.kt:176), comparing the 33-byte derived compressed pubkey against the stored publicKeyData — which for ECDSA_HASH160 (and EDDSA_25519_HASH160) rows is the 20-byte hash, so the comparison always fails and that supported key type can never be repaired. Make the ownership check key-type-aware (ideally in Rust, matching DPP's comparison semantics) and add a real HASH160 repair test.
  2. P1 — KeyStoreException numeric code 4 is ERROR_INTERNAL_SYSTEM_ERROR, not a lock-screen signal (code 3 is the documented no-LSKF/auth-bound-key condition). As classified, a transient internal Keystore error during generation still silently downgrades AUTH_GATED → DEVICE_BOUND. Remove 4 from the downgrade classifier (handle OEM-internal signatures separately) and add a transient-code-4 negative test.
  3. P2 — the sign-time legacy invalidation path still converts durable-bookkeeping failure/cancellation into success via nested runCatching (PlatformWalletManager.kt:385): keep the typed code-31 outcome, but preserve a retryable repair signal instead of clearing it.

Nit: several new comments carry fix-round narration (issue/finding/reviewer provenance) — keep the invariant text, move the provenance to the PR description.

bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 23, 2026
…re (dashpay#4183)

The repair path derived the KEYPAIR and required its public half to equal the
key's stored on-chain data before persisting. For ECDSA_HASH160 /
EDDSA_25519_HASH160 keys DPP stores the 20-byte HASH160 of the pubkey as that
data, not the pubkey, so the raw 33-vs-20-byte contentEquals could never match
and those key types were permanently un-repairable.

derivedPublicKeyMatches now takes the DPP key-type discriminant and HASH160s the
derived pubkey (RIPEMD160(SHA256)) before comparing for HASH160 types; every
other type keeps the plain content comparison. keyType is threaded through
PrivateKeyDeriver.deriveAndStore; the repair path reads it from the persisted
row's breadcrumbs, the store path passes it from the persist callback. Adds a
pure-Kotlin Hash160 helper (public bytes only — no derivation/secrets, within
the CLAUDE.md doctrine) pinned to RIPEMD-160 reference vectors, and tests
proving a HASH160-type key repairs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 and others added 14 commits August 3, 2026 19:25
…s and verify the pubkey before persisting

Blocker 1 (shumkov): the repair path derived from caller-supplied indices
(the example app passed the DPP key id as keyIndex), and its only check was
probeIdentityKeyRecoverability — which proves the stored blob DECRYPTS, not
that the derived key is the RIGHT one. A wrong index derives a different
valid scalar that round-trips fine, clears pending state, and persists an
unusable key.

Fix:
- IdentityKeyPrivateKeyDeriver: on the force (repair) path derive the KEYPAIR
  and verify the derived public key equals publicKeyData BEFORE any store;
  a mismatch throws IdentityKeyDerivationMismatchException without persisting.
- PlatformWalletPersistenceHandler.repairIdentityKeyDurably: new orchestration
  that reads the derivation slot from the PERSISTED public_keys breadcrumbs
  (derivationIdentityIndex/derivationKeyIndex), never a caller index; a row
  without breadcrumbs fails the repair without clearing pending. Hoisted here
  (not the JVM-unconstructable manager) so it is unit-testable and shares the
  authoritative pendingIdentityKeys state.
- PlatformWalletManager.repairIdentityKey: drop the identityIndex/keyIndex
  params and delegate; supply only the wallet-scoped probe.
- WalletKeyHealthSheet: stop passing report.identityIndex / key.keyId.

Tests (PlatformWalletPersistenceHandlerTest): correct breadcrumbs derive,
verify, clear pending, and record the durable identifier (deriver called with
the persisted 3/5, not a caller index); mismatched breadcrumbs are rejected
(mismatch exception), nothing persisted, blob never probed, pending intact;
a row without breadcrumbs fails with SigningKeyUnavailable and never derives.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Blocker 3 (shumkov): in the repair path the Room privateKeyKeychainIdentifier
update was wrapped in runCatching (swallowed on failure) while the pending
state was cleared regardless. A failed durable write then resurrected the
repair after restart (the reconstruction reads the persisted rows) while the
live session believed it was done — session and restart disagreed.

Fix: the durable write in repairIdentityKeyDurably now fails CLOSED — it runs
through an injectable persistDurableIdentifier seam (default: the production
public_keys write) and its exception propagates; markIdentityKeyRepaired only
runs after the write commits. A failed durable write leaves pending intact and
the repair retryable, so the session and a restart agree.

Test: a repair whose derive + verify succeed but whose durable write throws
propagates the failure and leaves the pending entry in place.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…horitative signals

Blocker 2 (shumkov): isNoSecureLockScreenKeyGenFailure classified any
KeyStoreException whose message contained "generate_key" as a lock-screen
rejection. But "generate_key" names the failing operation, not its cause — a
transient KeyMint generation failure on a device WITH a lock screen carries it
too, so a transient failure would silently and PERMANENTLY downgrade an
AUTH_GATED key to DEVICE_BOUND instead of retrying.

Fix: drop the bare "generate_key" substring match. Classification now rests
only on the two authoritative signals — explicit lock-screen text, or the
lock-screen rejection numeric code (internal Keystore 4 / KeyMint 10309, which
the real android.security.KeyStoreException exposes via getNumericErrorCode()).

Tests: the on-device shape now classifies via the numeric code (not the
incidental generate_key text); two new negatives — a bare generate_key failure,
and a transient generate_key failure under a key-gen ProviderException with a
non-lock-screen numeric code — must NOT classify, so AUTH_GATED is retried, not
downgraded.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…olicy-alias migration

Finding 4 (shumkov): migrateToPolicyAlias wrapped its encrypt + DataStore edit
in runCatching, which swallows kotlin CancellationException along with genuine
rewrite failures. A coroutine cancelled during the migration's suspend points
was silently absorbed instead of unwinding — a structured-concurrency
violation.

Fix: replace runCatching with an explicit try/catch that rethrows
CancellationException; only genuine rewrite failures stay best-effort (the
recovered value is still returned; migration retries on the next read).

Test: a cancellation landing at the migration encrypt (via the fake keystore's
onNextPolicyEncrypt hook) now propagates out of retrievePrivateKey rather than
being swallowed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fallback

Finding 5 (shumkov): the DEVICE_BOUND documentation promised hardware-backed
storage while the implementation permits a software AndroidKeyStore fallback
(generation prefers StrongBox, falls back to the TEE, and finally to a
software-backed AndroidKeyStore key on devices with no secure element —
generateWithLockScreenDegradation never fails generation on a missing one).

Align the docs: "device-bound" means non-exportable AndroidKeyStore, not a
hardware-storage guarantee; backing is hardware-isolated only where the device
provides it. AUTH_GATED shares the same backing characteristics.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re (dashpay#4183)

The repair path derived the KEYPAIR and required its public half to equal the
key's stored on-chain data before persisting. For ECDSA_HASH160 /
EDDSA_25519_HASH160 keys DPP stores the 20-byte HASH160 of the pubkey as that
data, not the pubkey, so the raw 33-vs-20-byte contentEquals could never match
and those key types were permanently un-repairable.

derivedPublicKeyMatches now takes the DPP key-type discriminant and HASH160s the
derived pubkey (RIPEMD160(SHA256)) before comparing for HASH160 types; every
other type keeps the plain content comparison. keyType is threaded through
PrivateKeyDeriver.deriveAndStore; the repair path reads it from the persisted
row's breadcrumbs, the store path passes it from the persist callback. Adds a
pure-Kotlin Hash160 helper (public bytes only — no derivation/secrets, within
the CLAUDE.md doctrine) pinned to RIPEMD-160 reference vectors, and tests
proving a HASH160-type key repairs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dashpay#4183)

LOCK_SCREEN_KEYGEN_REJECTION_CODES included android.security.KeyStoreException
code 4, but 4 is ERROR_INTERNAL_SYSTEM_ERROR — a generic/transient fault, not
the no-secure-lock-screen signal (code 3). Classifying it as no-lock-screen
silently and permanently downgraded an AUTH_GATED identity key to the weaker
DEVICE_BOUND alias on a transient error.

Keep only the KeyMint-specific 10309 in the numeric set. A genuine transient
internal error now falls through and is rethrown by resolveIdentityKeysWriteAlias
as a retryable write failure instead of a security downgrade; the explicit
lock-screen message path still classifies real no-LSKF rejections. Adds a
negative test that code 4 does not classify, and repoints the existing
numeric-code tests at 10309.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ancellation (dashpay#4183)

onSigningKeyInvalidated wrapped the durable recordSigningKeyInvalidated write in
a bare runCatching {} that swallowed a failed OR cancelled write while the sign
still returned typed code 31 — the pending-repair signal was lost silently and a
cancelled signer scope was masked. Replace with a try/catch that rethrows
CancellationException and, on other failures, logs loudly and rethrows so the
signer's own best-effort guard (not this lambda) is the single place that treats
bookkeeping failure as non-fatal; the repair stays retryable (durable rows are
untouched; the next sign attempt / next load reconstruction re-runs it).

Also fix loadPersistedWallets: its bare runCatching around the suspend
reconstructPendingIdentityKeysFromPersistence swallowed CancellationException —
rethrow it (a best-effort reconstruction failure is still absorbed and logged).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ashpay#4183)

The WalletKeyHealthSheet recoverability probes (which decrypt each Keystore blob)
ran inside produceState on the composition/Main thread; wrap the whole report
build in withContext(Dispatchers.IO).

The state was named hasPrivateKey and every failed probe was labeled "Missing —
no Keystore entry", but the probe can't tell a truly-absent key from a present-
but-stranded/undecryptable one. Rename to isRecoverable and relabel
("Unrecoverable — key material missing or stranded; re-derive to repair", summary
"Unrecoverable key material") so the UI reflects recoverability, not presence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address the actionable review blockers on dashpay#4183
(keystore rework). Four blockers fixed; three items deliberately left
for human decision (see PR notes) because a correct fix is not a
minimal, verifiable edit.

thepastaclaw blockers:
- KeystoreManager.resolveIdentityKeysWriteAlias / effectiveKeySecurityPolicy:
  probe the CURRENT lock-screen state BEFORE accepting an existing
  auth-gated alias. Removing the secure lock screen permanently
  invalidates the auth-gated key's private half while Android retains its
  public half, so reusing the alias would encrypt new scalars under a key
  that can never decrypt (and silently bypass requireAuthGated). Both the
  write path and effective-policy reporting now degrade to DEVICE_BOUND on
  a now-lockless device. Test updated to assert the corrected semantics.
- PlatformWalletPersistenceHandler.reconstructPendingIdentityKeysFromPersistence:
  the suspend isPrivateKeyDecryptable probe was wrapped in runCatching,
  which swallowed CancellationException and returned normally. Rethrow
  cancellation; only genuine probe failures become an unusable result.
- PlatformWalletPersistenceHandler.onPersistIdentityKeyRemoval: a removed
  identity key stayed in pendingIdentityKeys as a phantom repair target.
  Stage a pending-clear delta with the round (published on commit,
  discarded on rollback) keyed by (identityId, keyId). Adds commit +
  rollback tests.

shumkov P1 (correctness):
- KeystoreSigner: the suspend onSigningKeyInvalidated callback was wrapped
  in runCatching (swallowing cancellation one layer too low), and the
  outer catch(Exception) would re-swallow a rethrow. Rethrow
  CancellationException at both sites so cancellation propagates instead
  of being masked as a native completion.

Validation: :sdk:compileDebugKotlin + :sdk:compileDebugUnitTestKotlin
clean; full :sdk:testDebugUnitTest 265 tests, 0 failures.

Not applied (need human decision, documented in the PR report):
- EDDSA (type-4) key repair: the resolver derive is ECDSA-only, so the
  Ed25519 public-key comparison can never match; a correct fix needs an
  Ed25519 pubkey derivation the Kotlin comparison path does not have
  (FFI change + native rebuild, or an Ed25519 impl matching dalek's seed
  convention).
- Numeric lock-screen classifier (10309): the reviewer's claim that
  getNumericErrorCode() cannot return 10309 contradicts the existing tests
  that assert exactly that; the correct Android mechanism (message parse /
  getInternalErrorCode / a different public constant) cannot be determined
  without the reviewer's exact comment, and a wrong guess risks a security
  downgrade misclassification.
- error.rs generic-signer misclassification: a classifier-only starts_with
  change does not close the hole because signer.rs emits a foreign
  generic-code message verbatim; a correct fix also needs producer-side
  marker escaping at the completion boundary (coordinated multi-file
  change, beyond a clean suggestion).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…4183 review)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion (+ code-31 classification) (dashpay#4183 review)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eration wrappers

Several public signing paths flattened every SDK failure into an
operation-specific string variant before the FFI boundary could inspect
it: token transfer -> TokenError, DPNS registration and document replace
-> InvalidIdentityData. A genuine SigningKeyUnavailable completion still
left the reserved machine prefix inside those strings, but the resulting
variant reached the FFI catch-all and flattened to ErrorUnknown, losing
the host's key-repair routing (code 31).

Add platform_wallet::error::preserve_signer_key_unavailable_or, which
keeps a structured key-unavailable signer failure verbatim under
PlatformWalletError::Sdk (the one shape the FFI maps to
ErrorSigningKeyUnavailable) and hands every other error to the caller's
stringifying wrapper unchanged. The check is structural and position-0
only -- never a substring sniff -- so a foreign signer that merely
mentions the marker is not misrouted into key repair. Route the three
named wrapper sites through it.

The pure-logic crate mirrors the reserved prefix rather than depend on
the FFI crate; a compile-time assertion in platform-wallet-ffi pins the
mirror byte-identical to rs-sdk-ffi's canonical constant, so drift is a
build failure rather than a silent code-31 regression.

Refs: dashpay#4183 review (thepastaclaw), dashpay#4060 finding 7

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…peration wrappers

The earlier fix rerouted three stringifying wrappers (token transfer, DPNS
register, document replace) through preserve_signer_key_unavailable_or so a
structured SigningKeyUnavailable (code 31) survives to the FFI. Their sibling
operation wrappers still flattened the same signer failure into TokenError /
InvalidIdentityData strings, dropping the typed discriminator.

Reroute every remaining signer-bearing stringifying wrapper the same way:
- Token ops: burn, mint, freeze, unfreeze, claim, destroy_frozen_funds,
  pause, resume, purchase, set_price, update_config
- Document ops: create, delete, transfer, set_price, purchase
- Credit ops: withdraw, transfer_credits, transfer_credits_to_addresses
- register_from_addresses (nested so the existing address-nonce promotion
  is preserved unchanged, code-31 checked first)

No unrelated error mapping changed. Paths that already preserve the SDK
error via map_err(Sdk) (sdk_writer put_document/send_contact_request,
invitation, identity registration) are untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bfoss765
bfoss765 force-pushed the port/v4.1/android-integration branch from 2cd9483 to 8387858 Compare August 3, 2026 23:27
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
Resolves the two blocking review findings and the live suggestions on dashpay#4261.

Blocking — advance dashpay#3968's reissue frontier to 38. Code 37 is allocated to
dashpay#4204, so the branch-specific guidance in the non-conforming section and in
the 27/28 detail section could still have sent dashpay#3968 to 37 and recreated the
collision with ErrorShieldedInviteAlreadyClaimed. Both references now say
38+, and both state that the reserved 28 and 30 are not available either.

Blocking — 28 and 30 were labelled RESERVED in the table and the frontier
paragraph but "free" in the code-30 section and the collision history. Rule 1
lets a contributor claim a gap this file marks free, so the two labels gave
two different allocation frontiers. Everything now says reserved-not-free and
cites rule 1 explicitly.

Open-PR inventory: verified 2026-08-03 against each PR's file list and the
error.rs at its head. Adds dashpay#3417, dashpay#3549, dashpay#3992 and dashpay#4243 (all touch the crate
without claiming an integer; dashpay#4243 maps new wallet errors onto the existing
ErrorInvalidParameter). Removes dashpay#4240 and dashpay#4251, whose heads touch no file
under this crate; removes dashpay#4258, merged on 2026-08-03; records dashpay#4264 as
closed with its work carried by dashpay#4243.

Also in this pass:

- dashpay#4204's Swift mirror is recorded as PARTIALLY fixed. At d78b940 the raw
  case and its init(ffi:) arm exist, but PlatformWalletError has no typed case
  and its exhaustive init(result:) has no arm, so the Swift package does not
  compile as written. Rule 5's Swift clause is not yet satisfied there.
- The preamble no longer claims duplicate discriminants are always silent. Two
  different names on one integer is an E0081 after a merge (that is how the
  code-32 collision surfaced); the silent case is a meaning moving to a new
  integer, or an un-updated host mirror.
- Code 13 ErrorArithmeticOverflow does have an in-tree producer
  (shielded_send.rs); the row said it had none.
- dashpay#3968 is no longer described as simply un-rebased: its head does contain the
  2026-08-01 base, it is behind the current base, and a rebase alone fixes
  nothing because git sees no conflict — the branch must edit its own enum.
- Provenance re-read on 2026-08-03 from GitHub rather than carried forward,
  now as a table with a note per PR. dashpay#4183 and dashpay#4184 were rebased onto
  5d68612 today and keep 31 and 29 respectively; their stale in-enum
  reservation comments were corrected in the same rebase, so the code-30
  section now lists dashpay#4204 as the only branch still carrying one.
- markdownlint: MD018 (18 occurrences) and MD004 (mixed bullet styles) are
  both clean; MD013 is down to long table rows only.
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
…otFound

Split out of dashpay#4183 (port of dashpay#4060) per review: the
unmanaged-identity read fixes are conflict-free on v4.1-dev and should
not be blocked on the Keystore rework.

The FFI's blanket Option -> result conversion reports an identity the
wallet does not manage as PlatformWalletFFIResultCode::NotFound (98),
so the Kotlin callers' zero-handle checks were dead and every local
read over an unmanaged identity threw instead of returning absence.

- Dashpay: route getManagedIdentity through
  translateManagedIdentityNotFoundToZero so contacts()/syncState()/
  payments()/sendContactRequest() treat "not managed" as null/empty/
  false; ErrorInvalidHandle still propagates.
- Sweep the remaining dead `== 0L` sites: ManagedPlatformWallet
  .inMemoryIdentityStates (one unmanaged/just-removed id no longer
  throws through the whole listing) and IdentityRegistration
  .contestedDpnsNames (the intended "identity is not managed by this
  wallet" NotFound now actually surfaces).
- platform-wallet-ffi: platform_wallet_get_managed_identity keeps the
  three outcomes distinct (classify_managed_identity_outcome) so a
  stale/removed wallet surfaces ErrorInvalidHandle and never
  masquerades as an unmanaged identity; unit tests pin both arms.
- DashSdkError: name the code (PLATFORM_WALLET_NOT_FOUND_CODE = 98);
  the 98 -> DashSdkError.NotFound mapping is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
Split out of dashpay#4183 (port of dashpay#4060) per review: the
typed signing error is conflict-free on v4.1-dev and should not be
blocked on the Keystore rework.

The KeystoreSigner "missing key" completion error travels as free text
through Rust and used to come back as an opaque PlatformWallet.Generic
(or WalletOperation) failure. It is now built from a shared
MESSAGE_MARKER constant and recognized on the Kotlin boundary as the
typed DashSdkError.PlatformWallet.SigningKeyUnavailable, so hosts can
route users to key repair instead of showing a generic error. The
marker is only consulted on the catch-all codes (6 / else), so the
dedicated retry-semantics types are never overridden.

Known limitation (kept as-is from dashpay#4183 by request): the discriminator
is message-text-based (message.contains on the marker); a structured
error code across the FFI boundary is follow-up work in the parent PR
line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
…as split

The inherited dashpay#4172 instrumented tests hardcode the legacy KEYS_ALIAS and
the single-alias API; after the alias split the default-policy storage
writes under KEYS_ALIAS_AUTH_GATED, so simulating keypair replacement by
deleting the legacy alias no longer invalidates anything (the CI failure
shumkov diagnosed on storeIfAbsentRederivesWhenTheKeysAliasKeypairWasReplaced).

- WalletStorageOwnershipTest: the replaced-keypair, rejected-blob, and
  no-regeneration probes delete/inspect KEYS_ALIAS_AUTH_GATED; the
  fingerprint-capture test uses encryptForIdentityKeysAlias (and pins the
  captured producing alias); the stale-invalidation-cleanup test uses the
  alias-parameterized deleteIdentityKeysAliasIfCurrentGeneration.
- KeystoreSignerInstrumentedTest: canSignWith rejection simulates
  replacement of the policy alias.

The five DashPayUnlockAndSyncTest / WalletManagerRoundTripTest failures
from PR dashpay#4183's CI run were NOT test or SDK defects: the branch predated
dashpay#4172's workflow hardening (screen_off_timeout / stayon / dismiss-keyguard
+ the hard deviceLocked=0 guard), so the emulator re-locked mid-run and
every MASTER_ALIAS operation threw InvalidKeyException. Rebuilding on the
current base inherits the fixed workflow; those tests are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
…-> 37 and mirror it (dashpay#4204)

32 is allocated to `ErrorTransactionBuild` (dashpay#4247, also
carried by dashpay#4256) in ERROR_CODE_REGISTRY.md (dashpay#4261). This variant took 32
without a registry row, so the two collide as a hard `E0081: discriminant
value 32 assigned more than once` the moment both land — reproduced on a
real integration merge, not hypothetical. 27-36 are all claimed (27
ErrorShutdownIncomplete via the merged dashpay#4268; 29 dashpay#4184; 31 dashpay#4183; 32/33
37 is the allocation frontier.

The code was also unmirrored on BOTH hosts, which is the more dangerous
half: Swift is exhaustive, so it surfaced as .errorUnknown and lost its
identity; Kotlin fell through to Generic(32), and in any tree carrying
"shielded invite already claimed" as "reservation wallet mismatch". That
matters on the claim-recovery path specifically — the error is raised from
four sites in shielded/operations.rs, three inside the recovery function.

Adds the typed Kotlin PlatformWallet.ShieldedInviteAlreadyClaimed (terminal,
inherited isRetryable = false), the Swift enum case + init(ffi:) arm, a
DashSdkErrorTest assertion pinning 37, and refreshes the stale Swift
reservation comment the registry asked the next toucher to drop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@bfoss765

bfoss765 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto v4.2-dev at 5d68612a45 — now MERGEABLE

Head is now 8387858016891417110e5ccca02fc9575e0fa6e5. The branch was CONFLICTING against #4268 (registry-owned coordinator lifecycle), which rewrote the FFI enum, manager.rs, persistence.rs, and their Kotlin/Swift mirrors.

Conflicts and how they were resolved — all in commit 22fd3e5507, in the two error-code mirrors:

ErrorSigningKeyUnavailable = 31 did not move, per ERROR_CODE_REGISTRY.md — 31 is still this PR's, and existing codes are never renumbered.

One deliberate content change. The in-enum comment claimed 27–28 were reserved for the deferred-payment reservation trio and that #4185 must renumber off 26. Both statements are stale: 27 is now merged ABI, and the trio moved to 34–36. It now records that 28 and 30 are reserved, that 29 is #4184's, and points at the registry. Comment-only — no discriminant changed.

The two Kotlin files #4268 also touched — PlatformWalletPersistenceHandler.kt and PlatformWalletManager.kt — merged without conflict, and #4268's ownership/release_fn documentation is intact in the rebased tree (spot-checked alongside WalletManagerNative.kt).

Verification at the rebased head:

  • cargo check -p platform-wallet-ffi -p platform-wallet -p rs-sdk-ffi -p rs-unified-sdk-jni — clean.
  • The set of files this branch changes relative to its base is the same set as before the rebase.

All four outstanding blocking findings were re-verified against the rebased code and answered on their threads.

bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 3, 2026
… sites

Three more live review findings, all verified against current PR heads.

Rule 5 named a `PlatformWalletResultCode.init(result:)` that does not exist —
`init(result:)` belongs to the downstream `PlatformWalletError`. As written, a
contributor could add the Swift raw case and the typed error handling and still
omit `PlatformWalletResultCode.init(ffi:)`, which is where the generated C
constant is recognised; that switch has a `default:` yielding `.errorUnknown`,
so the omission compiles and silently loses the code's identity before typed
handling sees it. Rule 5 now enumerates all three Swift sites and says how each
one fails: (1) the raw case, (2) the `init(ffi:)` arm — silent, and (3)
`PlatformWalletError` + its `init(result:)` arm — a hard compile error, since
that switch is exhaustive with no `default:`. That third failure is exactly
what dashpay#4204 is sitting on at `d78b940a03`.

dashpay#4196 is no longer blocked. Its head moved to `12492e8c54`, the restack onto
dashpay#4185 is done, dashpay#4185's head `8813e98533` is an ancestor, the trio reads
34/35/36, and the PR is MERGEABLE against v4.2-dev. Verified the numeric
references it owns were carried too: the `StaleReservationToken` KDoc and
`fromPlatformWalletNative` mapping in `DashSdkError.kt` both read 34, and the
V2 broadcast KDoc in `ManagedCoreWallet.kt` reads 34 with the rest symbolic.
`PlatformWalletError::StaleReservation` refers to the code symbolically and
never carried a number. The section is now a resolution rather than an open
item; the account of why the restack was hard is kept, since that was the
substance of the delay.

The code-30 sweep was overstated. "No PR anywhere defines a code 30" is false
for the surveyed heads — dashpay#4185 and dashpay#4256 both did; that was the allocation,
not a competing claim. It now reads "no PR unrelated to dashpay#4185 defines a code
30", which is the claim that actually supports the conclusion. The list of
branches carrying the stale consent-code reservation is corrected to dashpay#4183,
dashpay#4204 and dashpay#4256's pre-renumber rationale (dashpay#4247 was never one of them).

Provenance and the proposed table pick up dashpay#4196's new head. markdownlint
MD018/MD004 remain at 0.
QuantumExplorer pushed a commit that referenced this pull request Aug 4, 2026
…ingKeyUnavailable (split from #4183) (#4191)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer
QuantumExplorer merged commit 189a3ab into dashpay:v4.2-dev Aug 4, 2026
21 checks passed
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 4, 2026
…-> 37 and mirror it (dashpay#4204)

32 is allocated to `ErrorTransactionBuild` (dashpay#4247, also
carried by dashpay#4256) in ERROR_CODE_REGISTRY.md (dashpay#4261). This variant took 32
without a registry row, so the two collide as a hard `E0081: discriminant
value 32 assigned more than once` the moment both land — reproduced on a
real integration merge, not hypothetical. 27-36 are all claimed (27
ErrorShutdownIncomplete via the merged dashpay#4268; 29 dashpay#4184; 31 dashpay#4183; 32/33
37 is the allocation frontier.

The code was also unmirrored on BOTH hosts, which is the more dangerous
half: Swift is exhaustive, so it surfaced as .errorUnknown and lost its
identity; Kotlin fell through to Generic(32), and in any tree carrying
"shielded invite already claimed" as "reservation wallet mismatch". That
matters on the claim-recovery path specifically — the error is raised from
four sites in shielded/operations.rs, three inside the recovery function.

Adds the typed Kotlin PlatformWallet.ShieldedInviteAlreadyClaimed (terminal,
inherited isRetryable = false), the Swift enum case + init(ffi:) arm, a
DashSdkErrorTest assertion pinning 37, and refreshes the stale Swift
reservation comment the registry asked the next toucher to drop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Aug 4, 2026
…pay#4268 claimed

dashpay#4268 merged `ErrorShutdownIncomplete = 27` into the v4.2-dev
FFI ABI, colliding with this PR's `ErrorStaleReservationToken = 27`. Renumber
the deferred build/broadcast trio to the contiguous block 34-36, which sits
above every code currently claimed by a merged commit or an open PR:

  27  ErrorShutdownIncomplete         MERGED, dashpay#4268
  29  ErrorAssetLockInsufficientFunds dashpay#4184
  31  ErrorSigningKeyUnavailable      dashpay#4183, dashpay#4259
  32  ErrorTransactionBuild           dashpay#4247, dashpay#4256
  33  ErrorTransactionSigning         dashpay#4256

28 and 30 are vacated and return to the free pool. Applied across the Rust
enum, the FFI/JNI rustdoc, the Kotlin mapping + KDoc + tests, and the Swift
mirror (which has no compile-time cross-ABI check, so it was verified by grep).

Also addresses three review suggestions:

* `PlatformWalletInfo::generation` is now `pub(crate)`. It was publicly
  assignable through `state_mut()` / `state_mut_blocking()`, so downstream safe
  code could swap the `Arc` while `PlatformWallet` and `CoreWallet` kept the
  original — splitting the generation identity `Arc::ptr_eq` compares, which
  would make `is_current_generation()` reject a live wallet, turn
  generation-bound reservation cleanup into a no-op, and let teardown exclude
  through a different lifecycle gate than the payments it must fence. All
  construction and mutation sites are already inside the crate.

* `buildSignedPayment` now runs under `opWithCleanupOnCancellation`. Native
  finalization mints the token before the blocking JNI call returns, so
  `withContext`'s prompt-cancellation handoff could discard the completed
  `SignedCoreTransaction` and leave the reservation to the GC Cleaner or the
  TTL. The discarded result is now closed deterministically.

* Native code 26 (`ErrorTransactionBroadcastRejected`) no longer falls through
  to `PlatformWallet.Generic`. It maps to a dedicated
  `TransactionBroadcastRejected` subtype so callers can tell a definitively
  rejected, consumed-and-released payment (rebuild it) from an unrelated
  generic wallet failure, with its non-retry-in-place semantics pinned in
  `DashSdkErrorTest`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants